Skip to content

feat(harness): unify project sessions [Agent Map 02/15] - #820

Merged
ynadge merged 3 commits into
mainfrom
review/agent-map-02-project-sessions
Sep 6, 2026
Merged

feat(harness): unify project sessions [Agent Map 02/15]#820
ynadge merged 3 commits into
mainfrom
review/agent-map-02-project-sessions

Conversation

@ynadge

@ynadge ynadge commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Primary change type

  • Feature

Problem and motivation

Project sessions inherited planner/builder roles that changed their tools and lifecycle. Users need ordinary writable sessions with the same project capabilities and preserved conversation history.

Summary and scope

Derive one trusted project/user/session identity on creation and resume, migrate persisted records, and compose the same writable project prompt and capability checks for every project session.

Recognize persisted scope_unavailable failures so a conversation can resume after its trusted project scope is restored. Document the removed role fields in both session identity and tool telemetry.

How this increment fits

Every ordinary project session receives the same capabilities. Legacy bootstrap adapters remain until part 7; persisted proposal actors switch atomically in part 9.

Stack and review boundary

  • Part 02 of 15 in the Agent Map review stack; review this increment against its predecessor.
  • Base: review/agent-map-01-project-roots.
  • Current head: a787c58ef0ec20cb5282d797ee229a709c365f0e; 2,172 changed lines across 24 files, counting additions and deletions including tests.
  • Repackages the corresponding final behavior from #804, #811. Original code and review history remain preserved.
  • Complete coworker testing branch: fix/studio-onboarding-followups.
  • The stack remains unmerged. Dependent PRs target their predecessor, so their diffs do not repeat earlier increments.

Related work

Agent Map checkpoint SAP-3147; relevant work SAP-3148. This packaging follows the maintainer-approved 15-PR split.

Validation

Fresh local checks against a787c58ef0ec20cb5282d797ee229a709c365f0e:

pnpm build — passed (exit 0)
pnpm typecheck — passed (exit 0)
pnpm lint — passed (exit 0)
pnpm test — passed (exit 0)

Tests and documentation

Regression coverage: Identity migration, malformed or conflicting ownership, prompt parity, trusted create/resume, and scope loss followed by restart and recovery.

See part 15 for integrated browser, native CLI, and Mac journey validation. The checks above were run independently on this PR’s own commit.

Linux tests run with ordinary user filesystem permissions; the sandbox's extra ambient capabilities are dropped. Hosted CI and automated review are separate from these recorded local results.

Compatibility and release impact

  • Compatibility: Breaking for embedders: ProjectAgentSession exposes projectId, userId, and sessionId; role and assignment are removed. AgentMapToolEvent.role is removed. Persisted valid records migrate, retaining conversation identity.
  • Changeset: Included: .changeset/ordinary-project-session-identity.md

Security

  • No secrets, credentials, private user data, or unsanitized logs are included.
  • This PR does not publicly disclose a suspected vulnerability.

AI assistance

  • Codex assembled the implementation, addressed reproduced defects, supplied tests and documentation, inspected the diff, and ran the checks above. Reviews are handled by hosted PR automation.

Checklist

  • Read CONTRIBUTING.md; implementation follows the requested 15-PR split.
  • Description reflects this PR's actual predecessor-relative diff.
  • Relevant tests accompany the changed behavior.
  • Root build, typecheck, lint, and test evidence matches the final implementation; any documentation-only update is identified above.
  • Release/documentation treatment is explained above.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review — PR #820 (round 1)

No confidentiality findings: the diff adds no changesets, docs or fixtures naming a
non-public company, no internal hostnames, and no hardcoded internal defaults.

Findings

1. Resume now fails closed where it used to degrade — moving a project root bricks its session history

packages/harness/src/core/session-manager.ts (resume) + packages/harness/src/server/index.ts:1288

Previously, if resolveIdentityForPath(cwd) returned nothing the resolver returned undefined,
the identity was dropped and the session resumed without Agent Map tools. Now the resolver
throws when persisted is set and the project is missing or has a different id, and
resume() throws again on any mismatch — and the reconcile self-heal branch runs only when
persisted is falsy, so an existing session never recovers.

Failure: user renames or moves their project directory (or the marker is lost). Every prior
session whose cwd is under that root now answers 409 PROJECT_SESSION_SCOPE_UNAVAILABLE
forever, including ordinary coding sessions that merely happened to live inside a project.
Since this PR gives every in-project session an agentMapIdentity, the blast radius is all of
them, not just planner sessions. Suggest: drop the identity and resume read-only-of-map (old
behavior) for the "project no longer resolves" case, and reserve the hard error for an actual
principal/project conflict.

2. Migration rejection is permanent, and its whitelist already disagrees with the type shipped in the same diff

packages/harness/src/core/project-session-legacy-migration.ts:60 (parseBootstrapState)

ProjectBootstrapErrorCode (added here, exported from src/index.ts) has seven members, but the
failed branch validates against six — "scope_unavailable" is missing. A persisted
projectBootstrap carrying that code parses as nulloutcome: "rejected"
rejectedProjectSessionMetadata.add(id)resume() throws for the process lifetime. The set is
written only in init() and never cleared, so there is no repair path short of the user editing
sessions.json. Slot 5–7 is exactly what will start writing that code. Either add the member to
the whitelist now, or derive both from one constant array so they cannot drift.

3. legacyStoragePrincipal is not storage-only — it makes proposal attribution a constant

packages/harness/src/server/agent-map-mcp-tools.ts:105

The comment says "neither the capability nor tool authorization consumes these fields", which is
true, but AgentMapProposalService.actorFor persists role/assignment into ProposalActor, and
that is read on user-visible surfaces:

  • packages/harness/web/src/components/AgentMapInspector.tsx:107 renders "Map planner" vs
    "Agent builder · unplanned" from latest.actor.role.
  • packages/harness/web/src/lib/use-agent-map-entry.ts:206,303 emits author_role /
    assignment_kind telemetry from it.

Every proposal accepted after this lands is stored as agent-builder / unplanned, so the
inspector always prints one string and both telemetry dimensions become dead constants — while
AgentMapToolEvent.role and the capability-event role were removed. Pick one: either strip the
attribution row/telemetry dimensions in this increment too, or say in the comment that the stored
actor is a placeholder and that the inspector copy is stale until slot 9.

4. Published-package behavior and API change with no changeset anywhere in the stack

@sapiom/harness is public (v0.14.0, no private: true). This diff narrows
HarnessSession.agentMapIdentity from PlanningSessionIdentity to ProjectAgentSession
(shared/types.ts:220) — a consumer reading session.agentMapIdentity.role stops compiling —
adds three new exported types to src/index.ts, removes AgentMapToolEvent.role, and changes
user-visible behavior (planner sessions become writable, new session-start copy, resume can now
409). CONTRIBUTING §Changesets requires one when a published package's behavior or API changes.
The PR body defers it to "activation", which is fine only if some increment in the stack actually
carries it — name that increment, and mark the agentMapIdentity narrowing as Breaking with a
migration note when it does. The three new type exports also deserve a line of justification:
ProjectBootstrapMetadata looks like internal lifecycle state, not consumer contract.

5. Exported safety helpers with no production caller

packages/harness/src/core/project-session.ts:48,70

isProjectSessionDispatchAuthorized and isWithinCurrentProject are called only from
project-session.test.ts. The JSDoc advertises "an additional server-side launch/resume safety
check", but no server path performs a containment check — resolveAgentMapIdentity does its own
thing. Either wire it into create/resume in this increment or move it to the slot that uses it;
shipped-but-unwired security code reads as enforced when it is not. Same shape, lower stakes:
removeLegacyProjectSessionMetadata and legacyProjectSessionStateRoot have no callers, and
AGENT_MAP_PLANNER_SYSTEM_PROMPT (profiles/agent-map-planner.ts:9) is now referenced nowhere.

Nits

  • buildFocusedPlannerContext still takes onboardOnFirstResponse, but the delegate ignores it —
    dead parameter, and the onboarding sentence is silently dropped (core/planning-session.ts:142).
  • resolveAgentMapIdentity runs studioProjectCatalog.reconcile(...) twice for any non-project
    cwd; the first pass predates pendingProjectCwds.add(cwd), so it sees a strictly smaller
    candidate set than the second. One reconcile with the pending cwd already added would do, and
    each pass inspectAgentProjectMarkers every recent dir on the session-create hot path.
  • planning-session.test.ts:208 swaps expect(context).not.toContain("prompt") for a blank line
    rather than an equivalent assertion; the leak check it stood for is gone.

Verdict: request changes — findings 1 and 2 are user-facing resume failures with no recovery
path, and finding 3 leaves shipped UI copy asserting something the code no longer tracks.

@ynadge
ynadge force-pushed the review/agent-map-02-project-sessions branch from d7c42c1 to fb03529 Compare September 5, 2026 10:20
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review — PR #820 (round 2, follow-up)

Delta: fb035297 is the only push since round 1's d7c42c13 (that sha was force-pushed
away, so the earlier findings were re-verified directly against head). The new changeset is
clean on confidentiality — no non-Sapiom company, no internal host, no private link.

Fixed

  • update SDKs to use payment protocol data #2scope_unavailable added to the parseBootstrapState whitelist plus a
    restart→resume regression test. Residual: the list is still hand-maintained next to the
    7-member ProjectBootstrapErrorCode union, so the next code added re-opens the same
    permanent-rejection bug; derive both from one array.
  • langchain v1 support + fixes #4 — minor changeset landed, narrowing marked Breaking for embedders with a
    migration note. It does not mention the AgentMapToolEvent.role removal; add that line.

Not fixed

  • cleanup HTTP SDKs #1 server/index.ts:1299 — persisted identities still hard-throw
    ProjectSessionScopeUnavailableError when the project doesn't resolve, and the branch is
    sharper than round 1 said: for persisted the resolver never calls
    studioProjectCatalog.reconcile(), so a catalog that hasn't yet re-derived the root
    (the eviction case the base PR's own changeset calls out) 409s every in-project session,
    not just a genuinely moved root. The new test stubs resolveAgentMapIdentity wholesale,
    so this path is still untested.
  • New, same root cause: the changeset publishes "unavailable project scope prevents
    resume until the current owner and root binding are valid again" — for a moved or renamed
    root nothing restores it short of recreating the exact old path. Changeset text can't be
    edited after publish; either fix the behavior or soften the sentence.
  • langchain SDK config cleanup #3 agent-map-mcp-tools.ts:105 unchanged — AgentMapInspector.tsx:107 and
    use-agent-map-entry.ts still read a now-constant actor.role/assignment.
  • chore: update URLs, defaults, and LangChain v1.x docs #5 unchanged — isProjectSessionDispatchAuthorized, isWithinCurrentProject,
    removeLegacyProjectSessionMetadata, legacyProjectSessionStateRoot,
    AGENT_MAP_PLANNER_SYSTEM_PROMPT still have no production caller.
  • All three round-1 nits unchanged.

Nothing round 1 got wrong.

Verdict: request changes — #1 remains a no-recovery resume lock, now also asserted as
recoverable in unretractable changeset copy.

@ynadge
ynadge force-pushed the review/agent-map-01-project-roots branch from 34177a9 to a2c8d7f Compare September 5, 2026 11:57
@ynadge
ynadge force-pushed the review/agent-map-02-project-sessions branch from fb03529 to a787c58 Compare September 5, 2026 11:57
Base automatically changed from review/agent-map-01-project-roots to main September 6, 2026 22:19
@ynadge
ynadge merged commit a5e0302 into main Sep 6, 2026
2 checks passed
@ynadge
ynadge deleted the review/agent-map-02-project-sessions branch September 6, 2026 22:20
@ynadge ynadge mentioned this pull request Sep 6, 2026
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant